Skip to content

Antalya 26.6: Cache vended credentials for REST catalogs - #2156

Open
zvonand wants to merge 3 commits into
antalya-26.6from
feature/antalya-26.6/ClickHouse-ClickHouse-pr-107960
Open

Antalya 26.6: Cache vended credentials for REST catalogs#2156
zvonand wants to merge 3 commits into
antalya-26.6from
feature/antalya-26.6/ClickHouse-ClickHouse-pr-107960

Conversation

@zvonand

@zvonand zvonand commented Aug 3, 2026

Copy link
Copy Markdown
Member

Dropped from this backport: the AI dropped these surfaces rather than pulling in a missing prerequisite. Reviewers: confirm each is genuinely optional.

  • catalog auth-state invalidation of the vended credentials cache (cacheCredentials state-snapshot check and the commitSettingsChanges cache clear) - depends on the RestCatalog CatalogState/prepareSettingsChanges machinery not yet on antalya-26.6

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Add option to cache vended credentials for REST catalogs; add a setting vended_credentials_cache_ttl (seconds). 300 by default. 0 means no caching (ClickHouse#107960 by @zvonand).

Cherry-picked from ClickHouse#107960.


Now, new vended credentials are requested on each metadata request. This PR adds an (optional) cache for creds with configurable TTL.

zvonand added 2 commits August 3, 2026 19:39
Kept only the source PR's own additions in the conflicted regions; the
surrounding "theirs" context (stateless-worker ProfileEvents, the
`initializeOrLeaveUnavailable` catalog loading path, and the
`ALTER DATABASE ... MODIFY SETTING` / `CatalogState` machinery) belongs to
other upstream PRs that are not on antalya-26.6.

Adapted: DatabaseDataLake::getCatalog reads vended_credentials_cache_ttl from the plain `settings` member, because antalya-26.6 has no MultiVersion<DatabaseDataLakeSettings> database_settings
Dropped: catalog auth-state invalidation of the vended credentials cache (cacheCredentials state-snapshot check and the commitSettingsChanges cache clear) - depends on the RestCatalog CatalogState/prepareSettingsChanges machinery not yet on antalya-26.6
@zvonand zvonand added releasy Created/managed by RelEasy antalya-26.6 ai-resolved Port conflict auto-resolved by Claude labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Workflow [PR], commit [6f076f7]

@zvonand

zvonand commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@zvonand

zvonand commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

@blau-ai

@blau-ai

blau-ai commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

CI triage

Verdict: 11 red checks — 0 caused by this PR. Every failure is pre-existing on antalya-26.6, a known-flaky test, or an infra glitch in the CI tooling. None of them touch the files or code paths this PR changes (REST-catalog vended-credentials caching + Azure buffers). All build flavors that had a valid cache/config compiled the PR's own code cleanly.

Check Class PR-caused?
Build (amd_msan), Build (arm_msan) pre-existing base-branch build break
Unit tests (asan_ubsan), Unit tests (tsan) pre-existing test failure
Stateless tests (amd_debug, parallel) flaky (randomized settings)
Regression: iceberg_1, oauth, s3_export_part, s3_export_partition, swarms pre-existing (test-suite vs build version drift)
Grype Scan clickhouse-server infra (scanner crashed)

Build (amd_msan) / Build (arm_msan) — pre-existing base-branch break

FAILED: .../Iceberg/IcebergWrites.cpp.o
/ClickHouse/src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp:179:28:
error: use of undeclared identifier 'getDecimalScale'

IcebergWrites.cpp is not in this PR's diff. getDecimalScale is declared in DataTypes/DataTypesDecimal.h, which the file does not #include (and none of its included headers pull it transitively). It compiles everywhere else only because ccache serves a stale object; the msan flavor rebuilds from source and hits the real error.

Proof it's pre-existing: the base branch's own MasterCI run 31142308506 (started ~4h before this PR's run) fails with the identical getDecimalScale error at IcebergWrites.cpp:179.

This is what blocks all the msan-dependent jobs (they show as dropped, not run).

Fix (belongs on antalya-26.6, not this PR): add #include <DataTypes/DataTypesDecimal.h> to src/Storages/ObjectStorage/DataLakes/Iceberg/IcebergWrites.cpp. That one line unblocks msan for every PR against the branch. I can open it as a separate blau/* PR if you'd like — just say the word.

Unit tests (asan_ubsan) / Unit tests (tsan) — pre-existing

Both fail on the same gtest throwing Code: 1001, std::runtime_error, e.what() = rebuild failed, in the DiskAccessStorageRecovery access-storage suite. Deterministic across both sanitizers, no connection to DataLake/Azure. Not introduced here.

Stateless tests (amd_debug, parallel) — flaky

Single failure: 00071_merge_tree_optimize_aio timed out ([ FAIL ] 600.03 sec). CI's own minimizer reports "Passes without randomization. Confirmed: the failure is caused by randomized settings" and the runner logs "NOTE: Failed 1 tests - do not block pipeline, exit with 0." MergeTree AIO test, unrelated to this PR. Safe to re-run.

Regression: iceberg_1, oauth, s3_export_part, s3_export_partition, swarms — pre-existing (version drift)

Every one of these fails at the "feature support validation" gate, not in any actual scenario logic:

  • oauth → OAuth/JWT authentication is not supported by this build
  • swarms → Swarm object_storage_cluster is not supported by this build
  • s3_export_part / partition → Export merge tree part is not supported by this build
  • iceberg_1 → Swarm object_storage_cluster_join_mode ... not supported and Iceberg partition timezone ... not supported

The shared regression suite is newer than the 26.6 build and probes features not present in this flavour. These would fail identically on the base branch and are independent of vended-credentials caching (the iceberg/oauth runs never reach the credential path — they bail at the feature check).

Grype Scan clickhouse-server — infra

Not a vulnerability finding. The scan step crashed before producing output (jq: error: Could not open file result.json, can't open 'raw.log', preceded by a pip dependency conflict), so the status resolves to "An error occurred." The keeper and -alpine server Grype scans both passed with 0 high/critical. Re-run.


PR health check

The PR itself looks healthy. All build configs with a valid cache — amd/arm debug, release, asan_ubsan, tsan, binary — are green, as are Fast test, the targeted integration and AST fuzzer jobs. No failing check touches the changed files (DatabaseDataLake*, RestCatalog*, ICatalog.h, StorageCredentials.h, Azure*, ProfileEvents.cpp) or their code paths. Bottom line: nothing to fix in this PR to make CI reflect its correctness — the board is red for reasons that predate and stand apart from it. The only genuine bug in sight is the base-branch IcebergWrites.cpp missing include above.

One non-CI note for reviewers: the PR description already flags that vended-credentials cache invalidation (auth-state snapshot check + commitSettingsChanges cache clear) was dropped from this backport. That's a functional review item, not a CI item.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-resolved Port conflict auto-resolved by Claude antalya-26.6 releasy Created/managed by RelEasy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants